Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DP-1602 Add data pool export / batch import commands #133

Merged
merged 18 commits into from
Aug 4, 2023
Merged

Conversation

IvanGandacov
Copy link
Contributor

@IvanGandacov IvanGandacov commented Aug 1, 2023

Description

Ticket: https://celonis.atlassian.net/browse/DP-1602

Added data pool batch push support
Added data pool export version 2, that's compatible with the data pool batch push.
Updated the documentation.

As already discussed in our meeting, the implementation will be adjusted to be more user-friendly. (ticket created: https://celonis.atlassian.net/browse/DP-1576)
For now, this request should be good for the Batch Import UI.

Import Data Pool

node content-cli.js import data-pools --jsonFile ./initial.json --profile dev1

info:    Data Pools batch import succeeded!
info:    Batch import report: 
{
    "dataModelIdMappings": {
        "14ded083-82de-4ca1-80a4-9d1b8d9d8fd2": "de7f22a7-41cf-45a7-ac53-6c77afe8cafd",
        "ea0ebb32-5701-46e3-8015-ae04412a2e80": "b31fde18-ba3a-48a0-98b2-3e53add083db"
    }
}
node content-cli.js import data-pools --jsonFile ./initial.json --profile dev1 --outputToJsonFile

info:    Data Pools batch import succeeded!
info:    Batch import report file: batch_import_report_1cf4d888-2ce2-4c9d-ae50-57f84c8a9a9d.json

Export Data Pool

node content-cli.js export data-pool --id 850728cc-c679-4925-954a-87fb39abb12b --profile local 

info:    Data Pools export succeeded!
info:    Exported Data Pool: 
{
    "schemaVersion": "3.0.0",
    "jobs": [
        {
            "id": "33530cfb-835f-42fb-.....
.....
node content-cli.js export data-pool --id 850728cc-c679-4925-954a-87fb39abb12b --profile local --outputToJsonFile

info:    Data Pools export succeeded!
info:    Export file: data_pool_850728cc-c679-4925-954a-87fb39abb12b.json

Checklist

  • I have self-reviewed this PR
  • I have tested the change and proved that it works in different scenarios
  • I have updated docs if needed

@IvanGandacov IvanGandacov marked this pull request as ready for review August 1, 2023 12:17
}

public createPullManager(id: string, filename: string, pullVersion: number): DataPoolManager {
const dataPoolManager = this.createBaseManager(id, filename);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would we want to append ".json" if the user did not type it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Could be a way, but I don't see this as an absolute necessity for now. If we get customer requests on this maybe we could add it later.

AlbinHoxha
AlbinHoxha previously approved these changes Aug 1, 2023
src/content-cli-push.ts Outdated Show resolved Hide resolved
@fisgeci
Copy link
Contributor

fisgeci commented Aug 1, 2023

Also moving forward we are trying to migrate to a different architecture for the content-cli. We are using services to implement the functionality of each command. Could you migrate only the changes for batch push and pull to this approach?
It would be similar to this case.

@IvanGandacov
Copy link
Contributor Author

IvanGandacov commented Aug 2, 2023

Also moving forward we are trying to migrate to a different architecture for the content-cli. We are using services to implement the functionality of each command. Could you migrate only the changes for batch push and pull to this approach? It would be similar to this case.

I adjusted according to the new architecture. 👍
Please see the updated description: #133 (comment)

@@ -19,14 +18,22 @@ export class FileService {
}

public readManifestFile(importedFileName: string): Promise<ManifestNodeTransport[]> {
const manifest: ManifestNodeTransport[] = YAML.parse(fs.readFileSync(path.resolve(importedFileName + "/manifest.yml"), { encoding: "utf-8" }));
const manifest: ManifestNodeTransport[] = YAML.parse(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just prettifying

public static packages(program: CommanderStatic): CommanderStatic {
program
.command("packages")
.description("Command to import all given packages")
.option("-p, --profile <profile>", "Profile which you want to use to list packages")
.option("--spaceMappings <spaceMappings...>", "List of mappings for importing packages to different target spaces. Mappings should follow format 'packageKey:targetSpaceKey'")
.option(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just prettifying

request.post(this.resolveUrl(url) + "?" + querystring.stringify(parameters), this.makeOptions(contextService.getContext().profile, body), (err, res) => {
this.handleResponse(res, resolve, reject);
});
request.post(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just prettifying

request.get(this.resolveUrl(url), this.makeOptions(contextService.getContext().profile, null), (err, res) => {
this.handleResponse(res, resolve, reject);
});
request.get(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just prettifying

request.put(this.resolveUrl(url), this.makeOptionsJson(contextService.getContext().profile, JSON.stringify(body), "application/json;charset=utf-8"), (err, res) => {
this.handleResponse(res, resolve, reject);
});
request.put(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just prettifying

import * as commander from "commander";

export class ContextInitializer {
public static async initContext(): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied into its own class to avoid repeating code.

@IvanGandacov IvanGandacov changed the title DP-1602 Add data pool batch push support DP-1602 Add data pool export / batch import support Aug 2, 2023
@IvanGandacov IvanGandacov changed the title DP-1602 Add data pool export / batch import support DP-1602 Add data pool export / batch import commands Aug 2, 2023
@IvanGandacov IvanGandacov marked this pull request as ready for review August 2, 2023 15:33
fisgeci
fisgeci previously approved these changes Aug 3, 2023
};

ContextInitializer.initContext()
.then(loadCommands, loadCommands)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we calling the loadCommands twice here? I haven't seen this syntax before/

Copy link
Contributor Author

@IvanGandacov IvanGandacov Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's exactly as it was before, just instead of writting the () => { getCommands(); } twice, I put it into a variable.

@IvanGandacov IvanGandacov merged commit a6d5f4f into master Aug 4, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants